Skip to content

Add unit tests for fit_ellipse_to_points function Resolves #36 Adds…#61

Open
AdityaGupta716 wants to merge 2 commits intoneuroinformatics-unit:mainfrom
AdityaGupta716:add-fit-ellipse-tests
Open

Add unit tests for fit_ellipse_to_points function Resolves #36 Adds…#61
AdityaGupta716 wants to merge 2 commits intoneuroinformatics-unit:mainfrom
AdityaGupta716:add-fit-ellipse-tests

Conversation

@AdityaGupta716
Copy link

Description

This PR adds comprehensive unit tests for the fit_ellipse_to_points function in the derotation module.

What is this PR?

  • Feature addition
  • Bug fix
  • Test coverage improvement

Testing

The tests verify:

  1. Circular pattern: Tests correct center estimation for points arranged in a circle
  2. Horizontal ellipse: Tests correct center estimation for elliptical arrangements
  3. Minimum points: Tests behavior with exactly 5 points (minimum required)
  4. Error handling: Tests that ValueError is raised when fewer than 5 points are provided
  5. NaN handling: Tests that the function correctly handles and skips NaN values

All tests use synthetic geometric patterns with known centers to verify accuracy within specified tolerances.

Related Issues

Resolves #36

…matics-unit#36  Adds comprehensive tests to verify correct ellipse center estimation.

This file contains unit tests for the fit_ellipse_to_points function, verifying its behavior with various point patterns, including circular and elliptical arrangements, as well as edge cases with insufficient points and NaN values.
@lauraporta
Copy link
Member

Hi @AdityaGupta716, as said in the issue #36, this PR is a duplicate of #60 and you might want to describe how it covers the same edge cases, and/or join efforts with @AlgoFoe

@AdityaGupta716
Copy link
Author

AdityaGupta716 commented Feb 16, 2026

@lauraporta i can merge the useful parts of #60 into this PR instead of duplicating and reuse their noisy ellipse+circle tests and keep my edge case test(min points , few points error,NaN handling ) in the same file and will update when done

@AdityaGupta716
Copy link
Author

@lauraporta i am done with the changes it replaces the old minimal tests for fit ellipse to points with a much more readable suite that actually tests how the function is used in practice it generates realistic synthetic ellipse including rotated and noisy ones and checks that the fitted ellipse recovers the correct center eccentricity and orientation across a range if configuration not just one or two and also keeps behaviour from the earlier tests NaN AND VALUE ERROR

@lauraporta
Copy link
Member

Hi @AdityaGupta716, alright thank you, I will review your PR in the coming weeks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add unit tests to fit_ellipse_to_points

2 participants